Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linuxManualConfig: install GDB scripts #221707

Merged
merged 4 commits into from
Mar 20, 2023
Merged

linuxManualConfig: install GDB scripts #221707

merged 4 commits into from
Mar 20, 2023

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented Mar 17, 2023

Description of changes

linux is unusual in that we include its sources in an output. There's no point unpacking into /build when we're going to copy the sources into $dev later. Let's unpack directly into the final destination of the code, and save copying a whole kernel source tree (often across filesystems!).

This also means that Kbuild knows the location of the sources, which will allow us to install the GDB scripts — some scripts are generated, and some are not, so the generated ones end up in the build directory, accompanied by symlinks to the non-generated ones in the source directory.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@alyssais alyssais requested a review from K900 March 17, 2023 18:49
@alyssais alyssais changed the title linuxManualConfig: unpack directly into $dev @alyssais linuxManualConfig: install GDB scripts Mar 17, 2023
@alyssais alyssais changed the title @alyssais linuxManualConfig: install GDB scripts linuxManualConfig: install GDB scripts Mar 17, 2023
@ofborg ofborg bot requested a review from lovesegfault March 17, 2023 21:42
@alyssais
Copy link
Member Author

Looks like we're still missing the GDB entry point (vmlinux-gdb.py). All the rest of the scripts are installed into the build directory in the modules path, but not that one.

@alyssais alyssais marked this pull request as draft March 17, 2023 22:05
@alyssais alyssais force-pushed the linuxManualConfig-unpack branch 4 times, most recently from e5d7a8c to ca48963 Compare March 18, 2023 09:42
We can avoid the need to explicitly exclude it later if we just put it
somewhere else to begin with.
linux is unusual in that we include its sources in an output.  There's
no point unpacking into /build when we're going to copy the sources
into $dev later.  Let's unpack directly into the final destination of
the code, and save copying a whole kernel source tree (often across
filesystems!).

This also means that Kbuild knows the location of the sources, which
will allow us to install the GDB scripts — some scripts are generated,
and some are not, so the generated ones end up in the build directory,
accompanied by symlinks to the non-generated ones in the source
directory.
We've basically been reimplementing this — by default it contains
vmlinux, dtbs (on applicable architectures), modules, and architecture
specific stuff like $(KBUILD_IMAGE) and a couple of other
miscellaneous files.
These are required to debug kernel modules.  Since we're now able to
do that, there's another reason besides BTF to enable DEBUG_INFO, so
I've done that for pre-BTF kernel modules as well here.

For GDB to get configured correctly, vmlinux-gdb.py has to be two
directories up from scripts/gdb, and vmlinux has to be next to
vmlinux-gdb.py.  The least invasive way to satisfy these constraints
is to make vmlinux a symlink, which GDB will resolve before looking
for vmlinux-gdb.py.

Tested both ways of getting the scripts into GDB that I know of:

gdb /nix/store/7n77ijlxkxr6d613h02lr707kvjx6j1k-linux-6.1.19-dev/vmlinux \
    -iex 'add-auto-load-safe-path /nix/store/7n77ijlxkxr6d613h02lr707kvjx6j1k-linux-6.1.19-dev/lib/modules/6.1.19/build/vmlinux-gdb.py' \
    -ex 'lx-version' \
    -ex 'q'
gdb /nix/store/7n77ijlxkxr6d613h02lr707kvjx6j1k-linux-6.1.19-dev/vmlinux \
    -ex 'source /nix/store/7n77ijlxkxr6d613h02lr707kvjx6j1k-linux-6.1.19-dev/lib/modules/6.1.19/build/vmlinux-gdb.py' \
    -ex 'lx-version' \
    -ex 'q'

Also tested that the strip changes don't result in meaningful output
size changes (there's some small variation due to BTF data not always
coming out the same size, which is unrelated), and built every kernel
I can on x86_64 to make sure I'm not relying on build system behaviour
specific to newer kernels.
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's pretty nice, thank you :)

@Ma27 Ma27 merged commit e999433 into master Mar 20, 2023
@Ma27 Ma27 deleted the linuxManualConfig-unpack branch March 20, 2023 19:42
@alyssais
Copy link
Member Author

Might have been worth waiting to merge at the same time as some kernel updates to minimise rebuilds, but maybe it doesn't matter.

@vcunat
Copy link
Member

vcunat commented Mar 21, 2023

I believe this broke unstable-small channel by https://hydra.nixos.org/build/213428624

checking whether CONFIG_MODULES is defined... no
configure: error: 
                *** This kernel does not include the required loadable module
                *** support!

@vcunat
Copy link
Member

vcunat commented Mar 22, 2023

Also noticed: https://hydra.nixos.org/build/213443349

ln: failed to create symbolic link '/nix/store/mqc7dp01xxlbcf7bq1221hf7mbzwbap5-kbuild-6.1.20-merged/vmlinux': File exists

(edited the above references to point to a "more common" variant)

@K900
Copy link
Contributor

K900 commented Mar 22, 2023

#222522

@ghost
Copy link

ghost commented Jul 20, 2023

There's no point unpacking into /build when we're going to copy the sources into $dev later.

There is a point. The point is not to have circular references in the outpaths, since Nix forbids that.

This PR broke both powerpc and mips, and was incredibly frustrating to deal with because trying each fix requires a ~1hour build, and the end of which you get an unhelpful message from Nix about cycles so you need to inspect the failed build... but --keep-failed and remote builder protocol don't mix. I really don't think this was worth it just to save a line or two in somebody's .gdbinit file telling the debugger where the kernel sources are.

A much better strategy would have been to put the sources in their own separate output (either "-source" or else move everything else out of "-dev"), since we know the source code never has any references to anything else in /nix/store. Then cycles wouldn't be possible. I am not going to implement this though, because I am really tired of fighting with this issue.

Comment on lines -190 to -191
"vmlinux" # for "perf" and things like that
] ++ optional isModular "modules"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR inadvertently destroyed the ability to build a kernel without the modules, which is a much faster build.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kernelConf.target
"vmlinux" # for "perf" and things like that
] ++ optional isModular "modules"
++ optionals buildDTBs ["dtbs" "DTC_FLAGS=-@"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likewise with buildDTBs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants